spi: remove write-only and read-only traits. #461
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When introducing the Bus/Device split in #351, I kept the ability to represent "read-only" and "write-only" buses, with separate
SpiBusRead
,SpiBusWrite
buses. This didn't have much discussion, as it was the logical consequence of keeping the separation in the already existing traits (Read
,Write
,Transfer
, ...).Later, in #443, when switching from the closure-based API to the operation-slice-based API, this required adding
SpiDeviceRead
,SpiDeviceWrite
as well, because you can no longer put a bound on the underlying bus with the new API.This means we now have seven traits, which we can reduce to two if we drop the distinction. So, is the distinction worth it?
I've always been on the fence about it, now I'm sort of leaning towards no.
First, using write-only or read-only SPI is rare.
Second, for it to be useful HALs have to track "read-onlyness / write-onlyness" in the type signature, so a read-only SPI really only implements
SpiBusRead
and notSpiBus
. HALs already have enough generics. For example, Embassy HALs don't implement the distinction (you can create MOSI-only or MISO-only SPIs, but they all impl the fullSpiBus
, because I didn't want to make the types carry pin information).Third, it makes the API easier to use. Simpler, users only have to import one trait, docs have all the methods in one place. Much less boilerplate to impl the traits (look at how shorter
embedded-hal-bus
gets!).So I think we should remove them.